Add tdbg schedule audit command#10376
Conversation
| // - UnsupportedReason != "" -> reclassify real_miss to unsupported_policy and stamp the reason. These are | ||
| // corner-case policy/state configs the algorithm does not model correctly today, so we move the count out of | ||
| // the trusted real_miss bucket and surface the row for manual review. Reasons currently detected: | ||
| // - keep_original_workflow_id -- all fires share one WorkflowID, collapsing the chain-by-WorkflowID model. |
There was a problem hiding this comment.
this is in the API but not currently supported. It won't be for quite some time.
There was a problem hiding this comment.
gotcha, changed comment
|
I think input and output should be json/josnl. we have |
| NamespaceConcurrency int | ||
| } | ||
|
|
||
| func parseAuditInputs(c *cli.Context) (*auditInputs, error) { |
There was a problem hiding this comment.
could we support unix pipes instead of specifying an input file? the piped input could contain, namespace and schedule id (optional).
that way someone can stream the input from another process without writing a file and they ca cat a file into stdin.
There was a problem hiding this comment.
yup, that was on my todo list.
FILE FORMAT (for --file / stdin)
One audit target per line as 'namespace[,schedule_id]'. Examples:
checker-ses-northwest-prod.90d0d
datastore-northwest-prod.90d0d,DeleteExpiredSecretsScheduledWorkflow--one
synthetics-northwest-prod.90d0d,my-schedule
Lines starting with '#' and blank lines are ignored. Schedule IDs must not contain commas.
EXAMPLES
Single namespace, 1-day window, write CSV bundle:
tdbg schedule audit --namespace my-ns --start 2026-05-19T00:00:00Z --end 2026-05-20T00:00:00Z --output-dir ./audit-out
Many targets from a file:
tdbg schedule audit -f ./targets.csv --start 2026-05-01T19:30:00Z --end 2026-05-02T10:00:00Z \
--output-dir ./audit-out
Pipe targets from stdin (cat, psql, awk, etc.):
cat ./targets.csv | tdbg schedule audit -f - --start 2026-05-01T00:00:00Z --end 2026-05-02T00:00:00Z
| } | ||
| } | ||
|
|
||
| // expectedFireTimes returns the nominal (pre-jitter) fire times the spec would produce in (start, end]. Uses the |
There was a problem hiding this comment.
Actually pre-jitter is intentional...workflows started by the scheduler carry TemporalScheduledStartTime set to the nominal (pre-jitter) time, so the audit needs nominal to match the fired vs expected. I found this to be most reliable. I actually removed the jitterseed as it's not relaly needed and in mislead. Added comment as well.
|
|
||
| // maxAuditWindow caps how wide a single audit window can be. Catches typos (e.g. wrong month in --end) and discourages | ||
| // expensive multi-day runs that should be chunked into separate invocations. | ||
| const maxAuditWindow = 7 * 24 * time.Hour |
There was a problem hiding this comment.
I think this should be overridable. some ns may have schedules that run once a year.
There was a problem hiding this comment.
ah yea good point. Turned it into a flag, and added warning if exceeds 7days
New `tdbg schedule audit` command that detects missed schedule fires by comparing expected fires from each schedule's spec against actual workflow executions in visibility. Reports per-schedule classification (real_miss / skip_overlap / inconclusive_schedule_changed / unsupported_policy) as CSV, either bundled per-namespace to a directory or streamed flat to stdout.
fretz12
force-pushed
the
fredtzeng/tdbg-schedule-audit
branch
from
d13ec41 to
a0f8f3f
Compare
| if d := in.WindowEnd.Sub(in.WindowStart); d > defaultMaxAuditWindow { | ||
| _, _ = fmt.Fprintf(os.Stderr, "warning: window is %s (longer than default cap %s); proportionally slower and more memory-intensive\n", | ||
| d.Round(time.Hour), defaultMaxAuditWindow) | ||
| } |
There was a problem hiding this comment.
IMO this is a bit too verbose for a debug tool.
| // defaultMaxAuditWindow caps how wide a single audit window can be by default. Catches typos (e.g. wrong month in | ||
| // --end) and discourages expensive multi-day runs that should be chunked into separate invocations. Operators can | ||
| // raise the cap via --max-window for schedules that fire less frequently (quarterly, yearly). | ||
| const defaultMaxAuditWindow = 7 * 24 * time.Hour |
There was a problem hiding this comment.
I think it's a bit redundant to have both a --max-window and a --start-window option, if one can't work without also setting the other; how about droping the max, and setting a default start/end time that's [now-7d, now]?
| // we'd miss those workflows and falsely flag them as real_miss. 24h is generous for the patterns we've observed | ||
| // in practice; may need to raise if we encounter BUFFER chains that routinely run longer than a day. |
There was a problem hiding this comment.
I hadn't realized we had data on this already (how long buffer chains could be) - where have we observed this from?
| if policies.GetKeepOriginalWorkflowId() { | ||
| // All fires share one WorkflowID, breaking our chain-by-WorkflowID model. | ||
| reasons = append(reasons, "keep_original_workflow_id") | ||
| } |
There was a problem hiding this comment.
Weirdly, this setting has no effect in practice - known issue. We've considered introducing support for it, but you can leave this out for now (particularly since if a customer did have it set, it wouldn't be doing anything for them).
| case enumspb.SCHEDULE_OVERLAP_POLICY_BUFFER_ALL: | ||
| // Fires can be buffered for arbitrary durations; our delayedFireBuffer of 24h may miss the workflow. | ||
| reasons = append(reasons, "overlap_buffer_all") |
There was a problem hiding this comment.
tbh, I don't think this is much riskier in practice than hedging the 24h against BUFFER_ONE, I'd support it.
| // real_miss as skip_overlap when a long-running prior fire is active | ||
| // at the expected time. Surface for inspection; we don't reclassify |
There was a problem hiding this comment.
I'm not sure I quite follow; for this policy, if we're querying for started workflows based on the StartedByScheduleID SA, I'd expect us to have fired actions regardless of any existing running state. Since the WID is always unique per fire time (since GetKeepOriginalWorkflowId doesn't actually do anything), you can reasonably expect every time in a schedule's allow_all spec to match a fired WF.
| var jitterSecs int64 | ||
| if j := info.GetSpec().GetJitter(); j != nil { | ||
| jitterSecs = int64(j.AsDuration().Seconds()) | ||
| } |
There was a problem hiding this comment.
think we can drop this since we explicitly ignore jitter during processing
| var jitterSecs int64 | ||
| if j := spec.GetJitter(); j != nil { | ||
| jitterSecs = int64(j.AsDuration().Seconds()) | ||
| } |
There was a problem hiding this comment.
ditto, drop filling in jitter imo
| var rpcErr error | ||
| resp, rpcErr = l.client.ListSchedules(ctx, &workflowservice.ListSchedulesRequest{ | ||
| Namespace: namespace, | ||
| MaximumPageSize: 1000, |
There was a problem hiding this comment.
use visibilityPageSize?
|
|
||
| for ns, rs := range byNS { | ||
| // Sanitize "/" -> "_" | ||
| safeName := strings.NewReplacer("/", "_", string(filepath.Separator), "_").Replace(ns) |
3 participants
What changed?
Adds a new tdbg schedule audit subcommand under tdbg schedule. The command lists every schedule in the target namespace(s), computes the nominal fire times each spec should have produced inside a user-specified window, queries visibility for the workflows that actually ran, and emits a per-schedule classification of each expected fire:
Output is either a CSV bundle (summary.csv + per-namespace files) when --output-dir is set, or a single flat CSV stream to stdout when it isn't.
Why?
We've had several incidents where schedules silently missed fires, and answering "did the scheduler fire when it should have, during this window?" required ad-hoc visibility queries per schedule. This tool makes that question answerable in a single command across all schedules in many namespaces in parallel, and produces a machine-readable artifact that can be diffed across days to spot regressions.
How did you test it?